Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FreeBSD Java support #1014

Merged
merged 1 commit into from
Sep 7, 2021
Merged

Conversation

hackacad
Copy link
Contributor

Signed-off-by: hackacad [email protected]

Description

Adding support for FreeBSD userland Java (installe via port/pkg)

Issues Resolved

Adding reusable OS check ($OS) and location of Java on Darwin and FreeBSD

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: hackacad <[email protected]>
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 30b8142

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 30b8142

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 30b8142

@dblock
Copy link
Member

dblock commented Jul 27, 2021

start gradle check

JAVA="$OPENSEARCH_HOME/jdk.app/Contents/Home/bin/java"
elif [ $OS = "freebsd" ]; then
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why wouldn't we be using the packaged Java version on FreeBSD?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as for Node, Linux ELF isn't compatible and JAVA_HOME afaik not set by default on FreeBSD

Copy link
Member

@dblock dblock Jul 27, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see how this makes things better, but we do bundle a JDK under $OPENSEARCH_HOME, shouldn't we be then bundling a compatible JDK, and releasing a FreeBSD package instead of doing this? I'm thinking out loud, so bear with my FreeBSD ignorance.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best as far as FreeBSD is concerned is probably to ship a tarball which only contains the built java objects, and does not bundle the JDK (so a platform agnostic tarball).

It is not uncommon for FreeBSD ports to download such tarballs to build packages by just adding dependencies to the relevant ports (e.g. openjdk14, node), extracting this archive at the right location and providing utility scripts (e.g. rc.d init file).

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 30b8142
Log 353

Reports 353

@hackacad
Copy link
Contributor Author

hackacad commented Jul 27, 2021

This leads back to my first question in #1013 (comment)

If you want to build a downloadable package for FreeBSD as well this would make sense.

BUT it will be removed in an official FreeBSD port anyway. It’s the was it’s done there. (e.g. you can define every supported Java version and chose which one you want to use for it - https://wiki.freebsd.org/Ports/DEFAULT_VERSIONS )
Nobody wants prebuilt/-packaged dependencies on FreeBSD 🙂

@dblock
Copy link
Member

dblock commented Jul 27, 2021

This laeds back to my first question in #1013 (comment)

If you want to build a downloadable package for FreeBSD as well this would make sense.

BUT it will be removed in an official FreeBSD port anyway. It’s the was it’s done there. (e.g. you can define every supported Java version and chose which one you want to use for it - https://wiki.freebsd.org/Ports/DEFAULT_VERSIONS )
Nobody wants prebuilt/-packaged dependencies on FreeBSD 🙂

Ok, this is clear. I think this PR is the right way to move forward. Today we don't work on FreeBSD anyway out of the box, so this would make it better by working in both dev and even prod bundle (albeit trailing some incompatible JDK and/or node).

Because I don't know what I'm doing I'll ask @nknize for a second A-OK before merging.

@dblock
Copy link
Member

dblock commented Sep 7, 2021

I'm merging this.

@dblock dblock merged commit 268d93b into opensearch-project:main Sep 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants